Skip to content

fix(logger): caplog working with parent Logger #6847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 27, 2025
Merged

fix(logger): caplog working with parent Logger #6847

merged 4 commits into from
Jun 27, 2025

Conversation

amin-farjadi
Copy link
Contributor

@amin-farjadi amin-farjadi commented Jun 21, 2025 •
edited by leandrodamascena
Loading

Issue number: #6753

Summary

Caplog fixture was not capturing logs made by parent loggers (logger with no child=True arg).

Changes

SupressFilter is not applied to pytest's LogCaptureHanlder.

User experience

Users can now use caplog fixture to test their parent logger's behaviour

def test_parent_logger_with_caplog(caplog, service_name):
    caplog.set_level("INFO")
    logger = Logger(service=service_name)
    logger.info("testing, testing...")

    assert len(caplog.records) == 1
    assert caplog.records[0].message == "testing, testing..."

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change? No

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@amin-farjadi amin-farjadi requested a review from a team as a code owner June 21, 2025 00:25
@amin-farjadi amin-farjadi requested a review from anafalcao June 21, 2025 00:25
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 21, 2025
@amin-farjadi amin-farjadi mentioned this pull request Jun 21, 2025
Closed
@amin-farjadi amin-farjadi force-pushed the fix/6753/caplog_with_parent_logger branch 2 times, most recently from e1d5d1e to a571fa2 Compare June 22, 2025 16:26
@codecov Codecov
Copy link

codecov bot commented Jun 22, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.21%. Comparing base (a5482c8) to head (a571fa2).
Report is 1 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop    #6847   +/-   ##
========================================
  Coverage    96.21%   96.21%           
========================================
  Files          273      273           
  Lines        12691    12693    +2     
  Branches       946      947    +1     
========================================
+ Hits         12211    12213    +2     
  Misses         375      375           
  Partials       105      105

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leandrodamascena leandrodamascena linked an issue Jun 26, 2025 that may be closed by this pull request
Bug: Cannot use Pytest's caplog fixture with non-preconfigured Logger #6753
Closed
leandrodamascena
leandrodamascena approved these changes Jun 26, 2025
View reviewed changes
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @amin-farjadi thanks for sending this PR! I'm thinking that probably make sense I move everything to use caplog and I'll probably do that in a separate PR.

btw, the CI is failing and I'm trying to figure out.

APPROVED.

@amin-farjadi amin-farjadi force-pushed the fix/6753/caplog_with_parent_logger branch from 644447d to 3831250 Compare June 26, 2025 16:29
@boring-cyborg boring-cyborg bot added commons dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation github-actions Pull requests that update Github_actions code labels Jun 26, 2025
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 26, 2025
@amin-farjadi amin-farjadi force-pushed the fix/6753/caplog_with_parent_logger branch from 3831250 to 8735b26 Compare June 26, 2025 16:30
@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2025
@github-actions github-actions bot added bug Something isn't working and removed documentation Improvements or additions to documentation labels Jun 26, 2025
@amin-farjadi
Copy link
Contributor Author

@leandrodamascena
I think it would be good to keep some of the stream-based tests. When I was testing around to make my tests pass, my tests were passing but some of the stream-based tests were failing which, was quite useful...

@amin-farjadi
Copy link
Contributor Author

I will see why pytest is failing

@leandrodamascena
Copy link
Contributor

I will see why pytest is failing

Don't worry, this is about GH runners going crazy with threads. I'll fix it.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@leandrodamascena
Copy link
Contributor

Hey @amin-farjadi this is a temporary error that I'll fix that when refactoring part of our CI. I'll merge this PR.

@leandrodamascena leandrodamascena merged commit 149d7fa into aws-powertools:develop Jun 27, 2025
5 of 10 checks passed
@amin-farjadi
Copy link
Contributor Author

hey @leandrodamascena how does the tagging work? this fix didn't get tagged in develop 🤔

@leandrodamascena
Copy link
Contributor

hey @leandrodamascena how does the tagging work? this fix didn't get tagged in develop 🤔

Do you mean if we released to pypi or in the develop branch? It's here: https://github.com/aws-powertools/powertools-lambda-python/blob/develop/aws_lambda_powertools/logging/logger.py#L378

@amin-farjadi
Copy link
Contributor Author

I want to see which version of powertools I should pin my project to that has this fix in.

@leandrodamascena
Copy link
Contributor

I want to see which version of powertools I should pin my project to that has this fix in.

We will release a new version in the next week and then you can use it. For now it's only on the develop branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leandrodamascena leandrodamascena leandrodamascena approved these changes

@anafalcao anafalcao Awaiting requested review from anafalcao anafalcao is a code owner automatically assigned from aws-powertools/lambda-python-core

Assignees

@amin-farjadi amin-farjadi

Labels
bug Something isn't working commons dependencies Pull requests that update a dependency file github-actions Pull requests that update Github_actions code logger size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Cannot use Pytest's caplog fixture with non-preconfigured Logger
2 participants
@amin-farjadi @leandrodamascena